-
Notifications
You must be signed in to change notification settings - Fork 398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix up PHPStan errors. #1955
Fix up PHPStan errors. #1955
Conversation
Fixed conditions evaluating always to true / false Fixed unnecessary conditions Fixed common parts in if statement Fixed unnecessary concatenation Fixed immediately rethrown exception Fixed useless trailing commas
src/Propel/Runtime/ActiveQuery/SqlBuilder/AbstractSqlQueryBuilder.php
Outdated
Show resolved
Hide resolved
The other changes look good to me. |
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1955 +/- ##
============================================
+ Coverage 88.56% 89.27% +0.71%
+ Complexity 8051 8049 -2
============================================
Files 243 232 -11
Lines 24590 24519 -71
============================================
+ Hits 21777 21889 +112
+ Misses 2813 2630 -183
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
src/Propel/Runtime/ActiveQuery/SqlBuilder/SelectQuerySqlBuilder.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the mixed annotations, this looks great.
Really awesome improvements going forward! Thx :)
@mringler @dereuromark can we move forward here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, thank you for the reminder!
@michbeck It looks like @dereuromark has left the project, and it looks like there is no successor at Spryker at the moment. I have contacted the owner, and he told me that the project will keep being maintained by Spryker, but I haven't heard back if/when there will be someone who actively maintains Propel2, i.e. merges this PRs. |
@mringler thank you for the reminder and your patience! ❤️ I really hope you'll keep contributing after this, I'm really sorry that you had to wait, I know from personal experience that it is frustrating when your contributions won't move forward. 😞 I'll take this over temporarely until I figure out who will be our new internal owner. Your PR looks good to me! 💪 There is just one comment from Mark, to which I agree, that needs to be fixed but we could do that as well. @michbeck can you maybe take care of it tomorrow? I'll approve the merge as soon as this has been fixed by someone and I've got the required permissions for this repository. |
src/Propel/Runtime/ActiveQuery/SqlBuilder/SelectQuerySqlBuilder.php
Outdated
Show resolved
Hide resolved
@gechetspr this is ready for merge now. |
Road to level 8